-
Notifications
You must be signed in to change notification settings - Fork 3k
Hive: Fix some message typos in HiveCatalog: Matastore => Metastore #2950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| } catch (TException e) { | ||
| throw new RuntimeException("Failed to create namespace " + namespace + " in Hive MataStore", e); | ||
| throw new RuntimeException("Failed to create namespace " + namespace + " in Hive MetaStore", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, how do Hive folks prefer to write it? MetaStore or Metastore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly always used Metastore and it's like that in most of the Hive docs, but I've seen the camel case a lot too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve also always used Metastore.
Looking at this Hive design doc, it’s also Metastore with no capital S. I think this makes the most sense as it’s not really two words imo (and several Hive docs confirm it as being one word): https://cwiki.apache.org/confluence/plugins/servlet/mobile?contentId=27362072#Design-Metastore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say Metastore. Capitalizing the S was to match the class name.
kbendick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. I’m +0, only because I really feel it’s Metastore as one word and not MetaStore as two words.
I’ll let others chime in on that, but a basic search of docs etc seem to bring up Metastore as one word much more often than I’ve seen “official” docs using the phrase as two words.
|
|
||
| } catch (TException e) { | ||
| throw new RuntimeException("Failed to create namespace " + namespace + " in Hive MataStore", e); | ||
| throw new RuntimeException("Failed to create namespace " + namespace + " in Hive MetaStore", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve also always used Metastore.
Looking at this Hive design doc, it’s also Metastore with no capital S. I think this makes the most sense as it’s not really two words imo (and several Hive docs confirm it as being one word): https://cwiki.apache.org/confluence/plugins/servlet/mobile?contentId=27362072#Design-Metastore
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException( | ||
| "Interrupted in call to createDatabase(name) " + namespace + " in Hive MataStore", e); | ||
| "Interrupted in call to createDatabase(name) " + namespace + " in Hive MetaStore", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we go with Metastore and not MetaStore (which the former is correct according to a number of Hive and Cloudera docs), this also needs to be updated - plus several other locations :)
|
Yea I personally prefer Metastore as well due to Hive docs, I would guess the camel case comes originally from the Hive code itself (HiveMetaStore and HiveMetaStoreClient being the main classes..), so I didnt see the original casing as a problem. Anyway, removed camel case. |
kbendick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @szehon-ho!
|
Rebase to re-trigger test |
|
Test failure seemed flaky, try to fix it in : #2989 , rebase to trigger again |
Noticed a lot of these errors while trying to debug timeouted Hive tests